Tweak implicit resolution #6034
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have noted that the "more implicit parameters first" rule when applied
unconditionally can lead to implicit search explosion. The test case is
i3430.scala:
println(Nil.min) // error: ambiguous
We need to find an `Ordering[T]` for uninstantiated `T`. Before implicit arguments were taken
into account we'd try two types (in this case Long and BigInteger) and get an ambiguity. Then
we'd check whether any of the other candidates would dominate both of these types, which was
not the case. So, we are done with an ambiguity.
But when implicit arguments were taken into account there are many types that still are better
then these. E.g.
[T1: Ordering, T2: Ordering]: Ordering[(T1, T2)]
and so on far all other supported arities of Ordering for tuples! So one of these has to be tried.
That leads to searches for other uninstantiated orderings and so on. Running i3430.scala by hand,
I got a compiler hang. I am not sure why the test suite succeeded nevertheless; there must be
something surprising going on in the tests.
My fix to get round these issue is that now implicit parameters are only considered if the result
types of two alternatives are unifiable. Hopefully, that's not too constraining.
|
I solved the mystery why the neg/i3430.scala test was passing previously. We got: This PR contains a check file i3430.check that guards against relapsing in this way. |
|
I have to merge this now as it blocks other PRs. We should continue to brainstorm about better solutions. |
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 8, 2019
Looks like scala#6034 and scala#6040 collided resulting in a CI failure on master.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have noted that the "more implicit parameters first" rule when applied
unconditionally can lead to implicit search explosion. The test case is
i3430.scala:
We need to find an
Ordering[T]for uninstantiatedT. Before implicit arguments were takeninto account we'd try two types (in this case Long and BigInteger) and get an ambiguity. Then
we'd check whether any of the other candidates would dominate both of these types, which was
not the case. So, we are done with an ambiguity.
But when implicit arguments were taken into account there are many types that still are better
then these. E.g.
and so on far all other supported arities of Ordering for tuples! So one of these has to be tried.
That leads to searches for other uninstantiated orderings and so on. Running i3430.scala by hand,
I got a compiler hang. I am not sure why the test suite succeeded nevertheless; there must be
something surprising going on in the tests.
My fix to get round this issue is that now implicit parameters are only considered if the result
types of two alternatives are unifiable. Hopefully, that's not too constraining.